use correct LLVM intrinsic for min/max on floats by RalfJung · Pull Request #153343 · rust-lang/rust

RalfJung · 2026-03-03T11:40:46Z

The Rust minnum/maxnum intrinsics are documented to return the other argument when one input is an SNaN. However, the LLVM lowering we currently choose for them does not match those semantics: we lower them to minnum/maxnum, which (since llvm/llvm-project#172012) is documented to non-deterministically return the other argument or NaN when one input is an SNaN.

LLVM does have an intrinsic with the intended semantics: minimumnum/maximumnum. Let's use that instead. We can set the nsz flag since we treat signed zero ordering as non-deterministic.

Also rename the intrinsics to follow the IEEE 2019 naming, since that is mostly (and in particular, as far as NaN are concerned) now what we do. Also, minimum_number and minimum are less easy to mix up than minnum and minimum.

r? @nikic
Cc @tgross35
Fixes #149537

rustbot · 2026-03-03T11:40:50Z

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @oli-obk, @lcnr

Some changes occurred to the CTFE machinery

cc @oli-obk, @lcnr

RalfJung · 2026-03-03T11:41:14Z

compiler/rustc_codegen_llvm/src/intrinsic.rs

+                );
+                // `nsz` in minimumnum/maximumnum is special: its only effect is to make signed-zero
+                // ordering non-deterministic.
+                unsafe { llvm::LLVMRustSetNoSignedZeros(call) };


I have no idea if the way I wired up nsz is correct.^^

compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs

compiler/rustc_codegen_gcc/src/intrinsic/mod.rs

nikic

r=me once the questions for other backends are answered.

View changes since this review

nikic · 2026-03-03T12:02:48Z

compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp

+  if (auto I = dyn_cast<Instruction>(unwrap<Value>(V))) {
+    I->setHasNoSignedZeros(true);
+  }
+}


The C bindings have a native LLVMSetFastMathFlags(), we should probably switch to that. But I guess we should do that consistently for the existing LLVMRustSetAlgebraicMath/LLVMRustSetAllowReassoc/LLVMRustSetFastMath as well, so I don't particularly mind this in the meantime.

Yeah I don't know why we have a separate wrapper for each flag configuration here, but I figured I'd follow the existing pattern.

RalfJung · 2026-03-03T12:44:57Z

There are some odd things happening in CI

2026-03-03T12:31:32.4740066Z rustc-LLVM ERROR: Cannot select: 0xff67d41c22a0: f128 = fcanonicalize nsz 0xff67d41c2a10
2026-03-03T12:31:32.4740629Z   0xff67d41c2a10: f128 = AArch64ISD::CSEL 0xff67d41c5660, 0xff67d41c52e0, Constant:i32<11>, 0xff67d41c2930:1

Why did fcanonicalize end up with nsz? That was meant just for minimumnum.
And also it seems like f128 minimumnum is just broken on aarch64?

RalfJung · 2026-03-03T12:47:11Z

That was on the aarch64-gnu-llvm-20-1 runner. Maybe we have to fall back to minnum/maxnum for old LLVM versions?

library/core/src/intrinsics/mod.rs

nikic · 2026-03-03T13:23:58Z

That was on the aarch64-gnu-llvm-20-1 runner. Maybe we have to fall back to minnum/maxnum for old LLVM versions?

Ah yes, that's a good point. I believe minimumnum used to have some selection failures that were only fixed in LLVM 22.

RalfJung · 2026-03-03T13:46:04Z

I guess that makes sense, the test fails on old LLVM where we (have to) use the wrong intrinsic.

RalfJung · 2026-03-03T13:53:26Z

@bors try jobs=x86_64-gnu,aarch64-gnu

rename min/maxnum intrinsics to min/maximum_number and fix their LLVM lowering try-job: x86_64-gnu try-job: aarch64-gnu

RalfJung · 2026-03-03T15:27:27Z

This is very strange, I tested the fallback implementation locally and it passes. Why does it fail on the aarch runner?

And it's also a very strange return value. The inputs are from_bits(0x7fbfffff) and -9.0 and the output is from_bits(0x7fffffff)?!?

rust-bors · 2026-03-03T16:16:19Z

☀️ Try build successful (CI)
Build commit: 5fc4d3f (5fc4d3f9142818f2f2b292605ba61c2b9b55f112, parent: 1b7d722f429f09c87b08b757d89c689c6cf7f6e7)

RalfJung · 2026-03-03T16:27:48Z

now with the commit that always forces the fallback impl to be used
@bors try jobs=x86_64-gnu,aarch64-gnu,x86_64-gnu-gcc

rename min/maxnum intrinsics to min/maximum_number and fix their LLVM lowering try-job: x86_64-gnu try-job: aarch64-gnu try-job: x86_64-gnu-gcc

RalfJung · 2026-03-03T16:32:56Z

Seems like LLVM 20 straight-up miscompiles code like this

fn minimum_num(x: f32, y: f32) -> f32 {
    if x.is_nan() || y >= x {
        y
    } else {
        // Either y < x or y is a NaN.
        x
    }
}

const SNAN: f32 = f32::from_bits(f32::NAN.to_bits() - 1);

fn main() {
    dbg!(minimum_num(-9.0, std::hint::black_box(SNAN)));
}

I tried this on an aarch64 dev desktop: with Rust 1.87, an optimized build prints NaN, with latest stable Rust, it prints -9.0.

How do we handle library tests that trigger miscompilations on old LLVM versions...? We could just remove the black_box, but -- it'd be a shame to reduce test coverage on newer LLVM just because we also still test old LLVM.

Are we anywhere close to dropping LLVM 20? :D

RalfJung · 2026-03-04T08:55:11Z

For that try build I added a hack in the backend to always use the fallback impl. That's also what I did to reproduce this on the dev desktop.

nikic · 2026-03-04T09:01:38Z

The AArch64 miscompile is llvm/llvm-project#176624. And yes, that one still exists on current main. (The intrinsics are fine.)

RalfJung · 2026-03-04T09:32:22Z

@nikic thanks a ton, you just saved me a lot of digging. :)

RalfJung · 2026-03-04T09:36:40Z

So, for this PR -- I guess the best we can do is use the intrinsic on LLVM22+ and the fallback impl for older LLVM, even if the fallback impl sometimes gives the wrong results on aarch64 (for SNaN inputs). We already sometimes give wrong results on aarch64 before this PR so that's not even a regression.

Or can we get away with using the intrinsic on LLVM 21 already?

library/coretests/tests/floats/mod.rs

RalfJung · 2026-03-05T15:35:15Z

@bors try jobs=x86_64-gnu,aarch64-gnu

use correct LLVM intrinsic for min/max on floats try-job: x86_64-gnu try-job: aarch64-gnu

rust-bors · 2026-03-05T17:21:54Z

💔 Test for e9e3af0 failed: CI

RalfJung · 2026-03-05T21:12:18Z

"The operation was canceled"?

@bors try jobs=x86_64-gnu,aarch64-gnu

use correct LLVM intrinsic for min/max on floats try-job: x86_64-gnu try-job: aarch64-gnu

rust-bors · 2026-03-05T23:33:37Z

☀️ Try build successful (CI)
Build commit: e5a358e (e5a358e30e169fde99d9e8d64c89b29060241667, parent: 64b72a1fa5449d928d5f553b01a596b78ee255d2)

… lowering

RalfJung · 2026-03-06T10:24:26Z

I think this answers all questions regarding other backends, so as per previous review
@bors r=nikic rollup=never

rust-bors · 2026-03-06T10:24:29Z

📌 Commit 0fe2cf5 has been approved by nikic

It is now in the queue for this repository.

tgross35 · 2026-03-06T12:42:05Z

Adding f{min,max}imum_num* in rust-lang/compiler-builtins#1107 in case LLVM starts to emit the calls

RalfJung · 2026-03-06T12:48:08Z

@bors r-
Let's wait for that to sync then.

rust-bors · 2026-03-06T22:46:01Z

☔ The latest upstream changes (presumably #153507) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot assigned nikic Mar 3, 2026

RalfJung commented Mar 3, 2026

View reviewed changes

compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs Outdated Show resolved Hide resolved

RalfJung commented Mar 3, 2026

View reviewed changes

compiler/rustc_codegen_gcc/src/intrinsic/mod.rs Outdated Show resolved Hide resolved

nikic reviewed Mar 3, 2026

View reviewed changes

RalfJung force-pushed the min-max-fix branch from 685c6e9 to 0e8b777 Compare March 3, 2026 12:50

RalfJung commented Mar 3, 2026

View reviewed changes

library/core/src/intrinsics/mod.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

RalfJung force-pushed the min-max-fix branch from 0e8b777 to 3eb2e56 Compare March 3, 2026 13:52

This comment has been minimized.

Sign in to view

rust-bors bot pushed a commit that referenced this pull request Mar 3, 2026

Auto merge of #153343 - RalfJung:min-max-fix, r=<try>

5fc4d3f

rename min/maxnum intrinsics to min/maximum_number and fix their LLVM lowering try-job: x86_64-gnu try-job: aarch64-gnu

This comment has been minimized.

Sign in to view

RalfJung force-pushed the min-max-fix branch from 1b06b99 to 5b7ba5f Compare March 3, 2026 16:08

This comment has been minimized.

Sign in to view

rust-bors bot pushed a commit that referenced this pull request Mar 3, 2026

Auto merge of #153343 - RalfJung:min-max-fix, r=<try>

9c8cd8b

rename min/maxnum intrinsics to min/maximum_number and fix their LLVM lowering try-job: x86_64-gnu try-job: aarch64-gnu try-job: x86_64-gnu-gcc

RalfJung force-pushed the min-max-fix branch from 1ff1a00 to 87aca5f Compare March 4, 2026 09:39

This was referenced Mar 4, 2026

What are the intended semantics for simd_fmin/fmax and simd_reduce_min/max vs signaling NaN? #153395

Open

fmax optimization on aarch64 unexpectedly working for signaling NaN #151286

Open

RalfJung changed the title ~~rename min/maxnum intrinsics to min/maximum_number and fix their LLVM lowering~~ use correct LLVM intrinsic for min/max on floats Mar 4, 2026

nikic reviewed Mar 5, 2026

View reviewed changes

library/coretests/tests/floats/mod.rs Outdated Show resolved Hide resolved

RalfJung force-pushed the min-max-fix branch 2 times, most recently from 1504221 to 1e50a40 Compare March 5, 2026 15:34

This comment has been minimized.

Sign in to view

rust-bors bot pushed a commit that referenced this pull request Mar 5, 2026

Auto merge of #153343 - RalfJung:min-max-fix, r=<try>

e9e3af0

use correct LLVM intrinsic for min/max on floats try-job: x86_64-gnu try-job: aarch64-gnu

This comment has been minimized.

Sign in to view

rust-bors bot pushed a commit that referenced this pull request Mar 5, 2026

Auto merge of #153343 - RalfJung:min-max-fix, r=<try>

e5a358e

use correct LLVM intrinsic for min/max on floats try-job: x86_64-gnu try-job: aarch64-gnu

rename min/maxnum intrinsics to min/maximum_number and fix their LLVM…

0fe2cf5

… lowering

RalfJung force-pushed the min-max-fix branch from 1e50a40 to 0fe2cf5 Compare March 6, 2026 10:22

rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 6, 2026

rust-bors bot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 6, 2026

Uh oh!

Conversation

RalfJung commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Mar 3, 2026

Uh oh!

RalfJung Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nikic left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nikic Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Mar 3, 2026

Uh oh!

RalfJung commented Mar 3, 2026

Uh oh!

Uh oh!

nikic commented Mar 3, 2026

Uh oh!

This comment has been minimized.

RalfJung commented Mar 3, 2026

Uh oh!

RalfJung commented Mar 3, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

RalfJung commented Mar 3, 2026

Uh oh!

This comment has been minimized.

rust-bors bot commented Mar 3, 2026

Uh oh!

RalfJung commented Mar 3, 2026

Uh oh!

This comment has been minimized.

RalfJung commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung commented Mar 4, 2026 via email

Uh oh!

nikic commented Mar 4, 2026

Uh oh!

RalfJung commented Mar 4, 2026

Uh oh!

RalfJung commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

RalfJung commented Mar 5, 2026

Uh oh!

This comment has been minimized.

rust-bors bot commented Mar 5, 2026

Uh oh!

RalfJung commented Mar 5, 2026

Uh oh!

This comment has been minimized.

rust-bors bot commented Mar 5, 2026

Uh oh!

RalfJung commented Mar 6, 2026

Uh oh!

rust-bors bot commented Mar 6, 2026

Uh oh!

tgross35 commented Mar 6, 2026

Uh oh!

RalfJung commented Mar 6, 2026

Uh oh!

rust-bors bot commented Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

RalfJung commented Mar 3, 2026 •

edited

Loading

nikic left a comment •

edited by rustbot

Loading

RalfJung commented Mar 3, 2026 •

edited

Loading

RalfJung commented Mar 4, 2026 •

edited

Loading